-
Notifications
You must be signed in to change notification settings - Fork 615
fix(redis): correct span handling for multi commands #3140
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(redis): correct span handling for multi commands #3140
Conversation
2dbf32e to
fedac52
Compare
|
@pichlermarc Could you please take a look at this PR? I’ve updated the Redis instrumentation to handle MULTI and pipeline spans according to the semantic conventions. Specifically:
|
|
@blumamir Could you please take a look at this PR? |
pichlermarc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay.
I think the attribute should only be added to the span when the stability opt-in flag is set to stable since it's only part of the latest semconv (the old semconv is 1.7.0, it did not have db.operaration.name). Other than that this PR looks great 🙌
pichlermarc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
|
Thank you for your contribution @aryamohanan! 🎉 We would like to hear from you about your experience contributing to OpenTelemetry by taking a few minutes to fill out this survey. |
Which problem is this PR solving?
This PR fixes an issue in the Redis instrumentation for
MULTI(transaction) commands where spans were not correctly handled for mixed command sets.Previously, the
MULTIspan did not follow the semantic conventions and could not accurately represent it in the span dataAccording to this part of the semantic conventions:
Short description of the changes
Updated the
db.operation.nameon theMULTIspan:MULTI GETwhen all operations areGET.MULTIwhen commands are mixed, in compliance with the semantic conventions.Added support for
execAsPipeline, so that whenMULTIis executed withexecAsPipeline, it is treated as a pipeline in Redis instrumentation.Expanded tests to cover multiple command scenarios.
Related issue: #3082